Skip to content

security: enforce MCP scopes, fix SSRF bypass, add security context and URL awareness#12

Merged
mcheemaa merged 2 commits intomainfrom
security/hardening-and-url-awareness
Mar 31, 2026
Merged

security: enforce MCP scopes, fix SSRF bypass, add security context and URL awareness#12
mcheemaa merged 2 commits intomainfrom
security/hardening-and-url-awareness

Conversation

@mcheemaa
Copy link
Copy Markdown
Member

Summary

Security hardening and URL awareness improvements. Addresses findings from a comprehensive security audit conducted after the /trigger endpoint vulnerability (fixed in #11).

Security fixes

  • MCP scope enforcement. Tool-level scopes were fully implemented (TOOL_SCOPES, getRequiredScope(), hasScope()) but never wired into the request path. Read-only tokens could call admin tools like phantom_register_tool. Scopes are now enforced at the HTTP dispatch layer before the MCP transport sees the request. Handles batch JSON-RPC to prevent bypass.

  • IPv6 SSRF bypass. The URL validator's private IP check was bypassable via IPv4-mapped IPv6 addresses ([::ffff:127.0.0.1], [::7f00:1], [::ffff:0:7f00:1]). The URL parser strips brackets, so isIP() returned 0 and the entire check was skipped. Now handles mapped, compatible, and ISATAP forms.

  • Per-message security context. External channel messages (Slack, Telegram, Email, Webhook, CLI) get a concise security frame prepended and appended before reaching the agent. Reminds the agent not to leak credentials in responses. Internal sources (scheduler, trigger) bypass wrapping.

  • Session token removed from tool output. phantom_generate_login no longer returns the raw 7-day session token. The agent only sees the magic link URL.

  • MCP token log redaction. Runtime auto-generated tokens are no longer printed to stdout.

  • SWE tools added to scope map. phantom_review_request now correctly requires operator scope.

URL awareness

The agent sometimes didn't know its own public URL when asked to share UI links. Root cause: PHANTOM_DOMAIN was often missing from the environment, and the URL derivation was scattered across 5+ locations.

  • Added public_url config field with priority chain: PHANTOM_PUBLIC_URL env var > public_url in phantom.yaml > auto-derived from name + domain
  • Both env var and derived paths validate as proper URLs
  • System prompt tells the agent its public URL in identity, environment, and page sections
  • UI tools return full public URLs instead of relative paths
  • Health endpoint includes public_url
  • Backwards compatible: missing config works exactly as before

Test plan

  • 822 tests pass, 0 failures (28 new tests added)
  • Typecheck clean
  • Lint clean
  • MCP scope enforcement: read-only token blocked from phantom_ask and phantom_register_tool
  • Batch JSON-RPC bypass prevented
  • IPv6 bypass: all forms blocked (::ffff:, ::, ::ffff:0:)
  • Session token not in phantom_generate_login output
  • No raw tokens in runtime logs
  • URL awareness: PHANTOM_PUBLIC_URL, yaml config, name+domain derivation, no-domain fallback
  • Security wrapping: external channels wrapped, internal channels not wrapped
  • Reviewed by two independent Opus agents (SHIP IT)

…pping, URL awareness

Security hardening:
- Enforce MCP tool-level scopes at HTTP dispatch layer. The scope
  system (TOOL_SCOPES, getRequiredScope, hasScope) was fully
  implemented but never wired into the request path. Read-only
  tokens can no longer call operator/admin tools. Handles batch
  JSON-RPC to prevent bypass.
- Fix IPv4-mapped IPv6 SSRF bypass in URL validator. Handles mapped
  (::ffff:), compatible (::), and ISATAP (::ffff:0:) forms. Strips
  brackets from parsed hostnames before IP checks.
- Add per-message security context wrapping on external channels.
  Complements the system prompt with turn-level reminders about
  credential handling and social engineering resistance.
- Remove session token from phantom_generate_login tool output.
  Agent only needs the magic link, not the raw 7-day credential.
- Redact MCP tokens from runtime fallback logs in config.ts.
- Add all SWE tools to TOOL_SCOPES map.

URL awareness:
- Add public_url config field with priority chain: PHANTOM_PUBLIC_URL
  env var, then public_url in phantom.yaml, then auto-derived from
  name + domain. Validated as a proper URL at both paths.
- System prompt now tells the agent its public URL in identity,
  environment, and page instruction sections.
- UI tools return full public URLs instead of relative paths.
- Health endpoint includes public_url for operator verification.
- phantom init reads PHANTOM_PUBLIC_URL from environment.

28 new tests. 822 total, 0 failures.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2c541d1422

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +83 to +86
const wrappedText = this.isExternalChannel(channelId) ? this.wrapWithSecurityContext(text) : text;

try {
return await this.runQuery(sessionKey, channelId, conversationId, text, startTime, onEvent);
return await this.runQuery(sessionKey, channelId, conversationId, wrappedText, startTime, onEvent);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep security wrapper out of memory query text

handleMessage now forwards wrappedText into runQuery, but runQuery uses its text parameter for memoryContextBuilder.build() (which drives recallEpisodes, recallFacts, and findProcedure). For external channels, the retrieval query is therefore polluted with the same [SECURITY] boilerplate every turn, which degrades memory relevance and can surface unrelated context. Use the raw user message for memory lookup and only wrap the prompt that is sent to the model.

Useful? React with 👍 / 👎.

All version references updated to 0.18.2. Test count updated
from 785 to 822 (28 new security and URL awareness tests).
@mcheemaa mcheemaa merged commit 58571e1 into main Mar 31, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant